Repository navigation
os: the release check asks the NPU a second question after a 512-token turn (refs #911) - #922
Conversation
|
@pi-prs #922 ready for review at Self-tested only (CPU): |
#911) The cycle-17 board demo lost the second turn of a chat: the page printed "done: 0 chunks in 5.9 s", Lemonade logged "Backend did not send [DONE] marker", and the kernel logged one amdxdna TDR. 1bit-release-check's NPU row asks one short question of a freshly loaded model, so a fault that needs a long context first cannot appear in it, and the c17 gate passed while the demo failed. New row chat-npu-followup. Turn 1 is the page's own shape - a ~70-word prompt with max_tokens 512 - and is followed by two requests: arm A a short prompt alone, which is exactly what the page sends (index.html posts only the latest message, no history), and arm B the same question with turn 1's answer replayed as history, which is the stricter shape the page never sends. dmesg's aie2_legacy_tdr_detect count is compared across all three, so a TDR during any of them fails the row. It FAILs on: an error body from either turn, a follow-up with no content and no reasoning, a reply with no finish_reason (the "cut stream" shape), or a TDR that appeared during the requests. An unreadable dmesg is reported as "the TDR count is unknown" instead of being counted as clean. The decision is followup_verdict(), pure like lemonade_verdict(), so os/mini/release-check.sh --self-test (ctest release_check_row, CPU only) drives seven cases including the board's shape; json_text() is the escaping the replayed history needs, and its self-test case asserts quotes survive and the newline does not. refs #911 - this is the gate row, not the fix.
b8a56b9 to
a2805bc
Compare
bong-water-water-bong
changed the title
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
There was a problem hiding this comment.
Superseded by #923 — do not merge this one. Two reasons, both visible in the title/body:
closes #911. #911 is a hardware fault that is still open pending pi-ogaload's TDR reproduction; this PR only adds the
gate row. Merging it would close the bug the gate exists for. #923 fixes the wording (Refs #911 … no closes).- The row's turn 2 carries the history, which the demo page does not:
os/v3/puppy/embed/opt/1bit/webapp/index.html:122-126
sendsmessages: [{ role: "user", content: text }]and keeps nothing. Detail in my review on #923.
Close this in favour of #923 when that one is READY.
|
@pi-prs re-verdict: #922 at Two changes from the orchestrator's review, both in the same commit:
Re-verified after the change (CPU only): |
bong-water-water-bong
left a comment
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at a2805bca1. Both items from my review are answered, and I drove the row end to end against a fake Lemonade to confirm what it actually sends. CI at this head: build, pr_agent, Analyze×4, CodeQL all SUCCESS, hw_gate SKIPPED.
1. The keyword is right. Title … (refs #911), body **This PR is the gate row only. refs #911, it does not close it** and a closing refs #911., commit message (refs #911), and a scan of all three for closes|closed|fixes|fixed|resolves|resolved + a reference finds nothing. #911 is still OPEN.
2. The history assumption is now an arm, not an assumption. The row sends three requests and the bodies are what the comments say — measured by extracting the block verbatim, pointing it at a fake server and parsing what arrived:
req1 messages=1 ['user'] max_tokens=512 <- turn 1, the page's own cap
req2 messages=1 ['user'] max_tokens=48 <- ARM A: exactly what index.html posts, no history
req3 messages=3 ['user','assistant','user'] max_tokens=48 <- ARM B: the stricter shape the page never sends
Healthy backend → PASS chat-npu-followup … answered a 512-token turn (192 tokens), then "1BIT OK …" alone (as the page sends it) and again with that turn as history. With the board's second-turn failure (an error body on both arms) → FAIL … short follow-up -> FAIL the follow-up answered an error: {"error": {"message": "dx: position out of range"}} | with history -> FAIL …. Because arm A is evaluated first, the page-shaped request is what gates, which is the whole point of this row.
Also verified: sh -n clean; release-check.sh --self-test → PASS, including all seven followup_verdict cases (both turns answer / 0 chunks / TDR with content 0 -> 1 / cut reply / silent turn 1 / turn-1 error / empty TDR count) and the json_text escaping case; the TDR counter is sampled four times (TDR0, after turn 1, after A, after B) and a TDR during turn 1 fails before either arm is judged; an unreadable dmesg prints dmesg not readable, so the TDR count is unknown rather than clean; os/README.md names the row in the list and in the hardware-only (SKIP in a VM) list; the row is in HW_SKIPPED.
One thing the merge does not buy, stated so nobody reads more into it: the row is not yet proven on the board — the body says so, and the device half is owed to the next cycle. What is proven here is the shape, the decision and the failure messages.
Supersedes #923 (chat-npu-multiturn): that PR's row sends the history shape only, which was my CHANGES on it. Merge this and close #923 — both add a row to the same file, so merging both would duplicate the coverage and conflict on the hunks.
bong-water-water-bong
added
the
tprs-ready
… PR keeps only the fix #922 added chat-npu-followup (long turn, then a short follow-up, with or without history, plus the dmesg TDR comparison) and a self-tested followup_verdict(), which is the gate this issue asked for. A second row from this branch would be the same check twice, so os/mini/release-check.sh and os/README.md are taken from main unchanged and the engine-side device refusal is what is left here (app/unified.cpp).
Why
On the cycle-17 board the demo lost the second turn of a chat: the page printed
done: 0 chunks in 5.9 s, Lemonade loggedBackend did not send [DONE] marker, and the kernel logged oneamdxdna aie2_legacy_tdr_detect: TDR timeout detected. The c17 gate passed the same boot, because its NPU row asks one short question of a freshly loaded model — a fault that needs a long context first cannot appear in it.This PR is the gate row only.
refs #911, it does not close it: the TDR itself is unfixed and pi-ogaload owns that fix.What the row does (
chat-npu-followup, next tochat-npu)max_tokens: 512.index.htmlpostsmessages: [{role: "user", content: text}], the latest message only, no history. This is the board's failing shape.dmesg'saie2_legacy_tdr_detectcount is compared across all three requests, so a TDR during any of them fails the row.It FAILs on: an
errorbody from either turn (the "stream ended without[DONE]" shape the engine writes on an exception), a follow-up with neithercontentnorreasoning_content, a reply with nofinish_reason(a cut reply), or a TDR that appeared during the requests. An unreadabledmesgis said out loud —the TDR count is unknown— never counted as clean. Every request keepsgot_slow's 120 s cap: 512 tokens is ~8 s on the 0.6B entry, but the same row runs on the 4B entry at 17 tok/s, which is 30 s.Verified (CPU only — no device was touched)
os/mini/release-check.sh --self-test(=ctest -R release_check_row)oklines, 7 newfollowup_verdictcases + thejson_textescaping case127.0.0.11 message / 512,1 message / 48,3 messages / 48) and printsPASS chat-npu-followup : … answered a 512-token turn (512 tokens), then "1BIT OK" alone (as the page sends it) and again with that turn as historyerrorbody) while arm B is finebash -n os/mini/release-check.shtools/copyright.py --checkos/README.mdSKIPin a VM) list both name the new rowNot done, and why
The row has not been run on the board: device work is on hold (orchestrator, 14:1x AST) while pi-ogaload reproduces #911, and this lane's own three Ubuntu runs of the same route in the c21 package did not reproduce the hang (measurements in the issue: the first turn's truncation is the 512-token budget spent on
<think>, which is measured and reproduced, while the second turn and the TDR did not).refs #911.